Skip to content

Fix Poetry venv discovery to match Poetry (#327, #329) - #330

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-poetry-venv-discovery
Oct 1, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-poetry-venv-discovery

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Sep 30, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #327
Fixes #329

Summary

scan --mode agent now finds the virtualenv Poetry actually installed into in four cases it used to miss. In each case the scan had fallen back to the wrong interpreter, skipped the patch as package_not_installed, and exited 0:

  • a nameless package-mode = false project;
  • a Poetry 2 project that sets both [project].name and [tool.poetry].name;
  • an explicit virtualenvs.in-project = false next to a stray ./.venv;
  • every default Windows project.

Changes, all in crates/socket-patch-core/src/crawlers/python_crawler.rs:

  • Env name: poetry_project_names returns candidates in poetry-core's order: [project].name (2.x), then [tool.poetry].name, then non-package-mode. The first candidate that has a <name>-<hash>-py* dir on disk wins, so a Poetry 1.8 project (which ignores [project]) still finds its legacy-named env.
  • In-project precedence: find_local_venv_site_packages asks Poetry's layered config first, following EnvManager.use_in_project_venv: an explicit in-project decides, and only when it is unset does an existing ./.venv count. When Poetry wouldn't use ./.venv, its out-of-tree env is probed before ./.venv and ./venv. Those stay as fallbacks when Poetry has no env.
  • Windows hash: poetry_normalized_cwd goes through the new windows_normcase, which strips the \\?\ / \\?\UNC\ verbatim prefix as Python's realpath does, then lowercases. Pipenv's path normalization now shares the same strip_windows_verbatim_prefix helper.
  • Docs: docs/testing/poetry-compatibility.md ("Mode notes") and a CHANGELOG Fixed entry.

The existing unit test poetry_project_names_prefer_tool_poetry_and_return_both_spellings asserted the old, wrong precedence. It is replaced by poetry_project_names_follow_poetry_core_precedence.

Root cause

find_poetry_virtualenv_site_packages rebuilds Poetry's virtualenv location without running Poetry, and it differed from Poetry in three places:

  • It took the env name from [tool.poetry].name first and had no non-package-mode fallback. poetry-core 2.x's Factory uses project.get("name") or poetry_config.get("name", "non-package-mode"), and 1.9 uses local_config.get("name", "non-package-mode").
  • It let ./.venv win unconditionally, even when Poetry's config said in-project = false.
  • It hashed std::fs::canonicalize(cwd), which on Windows is \\?\C:\…. Poetry hashes normcase(realpath(cwd)), which has no such prefix.

Not in scope: the issues also point out that the scan exits 0 when the patch is skipped as package_not_installed. That's how agent mode handles any package that isn't installed, not something specific to Poetry. With discovery fixed, the patch applies in each of these cases.

Merge with v5 consolidation (#277), 2026-10-01

#277 landed on main and conflicted in python_crawler.rs and CHANGELOG.md. Merge commit df6cfba:

Re-validation after the merge:

  • cargo test -p socket-patch-core --all-features --lib -- crawlers::python_crawler: 46 passed.
  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo fmt --all -- --check: this PR's code is clean. main itself has many pre-existing rustfmt diffs (e.g. apply.rs, scan/*.rs), and CI doesn't run fmt.
  • cargo test --workspace --all-features --no-fail-fast: 9328 passed, 12 failed. All 12 are sandbox-only write-failure or permission tests that fail because the sandbox runs as root (*_write_failure_*, *unremovable*, relax_loop_must_not_traverse_symlinked_root, …). None of them are in the crawler.
  • Poetry e2e locally (Poetry 2.1.1): vendored passed. Hosted failed only at rollback's GET https://pypi.org/pypi/six/1.16.0/json, because the binary's TLS stack doesn't trust the sandbox proxy CA (curl to the same URL returns 200). CI's e2e_vex_build poetry:: legs passed on df6cfba for 1.0.10, 1.1.15, 1.8.5, 2.0.1 and 2.4.3 (Linux) and 2.4.3 (macOS).
  • CI on df6cfba: all 314 checks finished: success, apart from 4 that were skipped by design (e2e-docker, canary, downgrade, …). That includes test (ubuntu/macos/windows-latest), test-release and clippy. Bugbot's review of df6cfba found no issues. No conflict with main.

Test evidence (original change)

Issue Case Regression test Red on main Green
#327 1. nameless package-mode = false poetry_project_names_follow_poetry_core_precedence, poetry_out_of_tree_virtualenvs_are_discovered_without_a_dot_venv (nameless step) ✅ fails ✅
#327 2. [project].name + [tool.poetry].name same two tests (pep-name step) ✅ fails ✅
#327 3. in-project = false + stray .venv poetry_out_of_tree_virtualenvs_are_discovered_without_a_dot_venv (in-project step) ✅ fails (returned proj/.venv/...) ✅
#329 Windows \\?\ hash windows_normcase_drops_the_verbatim_prefix_like_python_realpath; #[cfg(windows)] poetry_normalized_cwd_has_no_verbatim_prefix_on_windows ✅ fails ✅ (incl. CI test (windows-latest))
  • Real Poetry 2.3.3 (Linux): I created real envs with poetry env use. find_local_venv_site_packages returned exactly the env poetry env info -p reports for the nameless, both-names and in-project = false projects.
  • The npm wrapper only dispatches, so it needs no change. The PyPI and gem wrappers were removed in v5 prerelease: scan → vex → vendor workflow, hosted by default #277.

🤖 Generated with Claude Code

https://claude.ai/code/session_017mSDqYENPT34fbMjLSQzBg


Note

Medium Risk
Changes which Python site-packages agent mode targets for patching; mis-discovery would skip or mis-apply patches, though the change is read-only discovery with extensive regression tests.

Overview
Agent mode (scan --mode agent) now locates Poetry’s real out-of-tree virtualenv in setups where discovery used to pick the wrong interpreter, skip patches as package_not_installed, and still exit 0.

In python_crawler.rs, venv probe order mirrors Poetry’s EnvManager.use_in_project_venv: when Poetry would not use ./.venv (explicit virtualenvs.in-project = false, or no in-project dir), Poetry’s cache env is checked before local ./.venv / ./venv, so stray local dirs from other tools no longer shadow Poetry’s install.

Env naming follows poetry-core: [project] name (2.x), then [tool.poetry] name, then non-package-mode; the first candidate with a matching <name>-<hash>-py* directory wins (Poetry 1.8 still finds legacy-named envs).

On Windows, cwd hashing strips the \\?\ verbatim prefix from canonicalize, matching Python’s realpath/normcase (shared helper with Pipenv path normalization).

CHANGELOG and docs/testing/poetry-compatibility.md document the behavior; unit/integration tests cover the #327/#329 cases.

Reviewed by Cursor Bugbot for commit 50cde0a. Configure here.


Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5
Agent mode missed the virtualenv Poetry installed into for nameless
package-mode = false projects, Poetry 2 projects naming both
[project] and [tool.poetry], projects with in-project = false next
to a stray .venv, and every project on Windows. The scan then
skipped the patch and exited 0.

Pick the env name in poetry-core's order ([project].name, then
[tool.poetry].name, then non-package-mode), honour an explicit
in-project setting before ./.venv, and drop the \\?\ prefix before
hashing the cwd on Windows.

Fixes #327
Fixes #329

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review September 30, 2026 15:59
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

Resolve conflicts with #277: keep Poetry-aware venv probe order in
find_local_venv_site_packages and add the Poetry entry to the
consolidated changelog.

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 1, 2026
@mikolalysenko

Mikola Lysenko (mikolalysenko) commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator Author

[burn-down agent] Ready for review on df6cfba: mergeable with current main (post-#277), CI all green (no failures; only path-filtered skips), Cursor Bugbot reviewed df6cfba with no new issues, no unresolved review threads. Labeled Ready for review.


Generated by Claude Code

Union the CHANGELOG Fixed entries from both sides.

Co-Authored-By: Claude <noreply@anthropic.com>
@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit 5678b76 into main Oct 1, 2026
35 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-poetry-venv-discovery branch October 1, 2026 13:27
@mikolalysenko Mikola Lysenko (mikolalysenko) removed the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 1, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

bugbot run


Generated by Claude Code

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 1, 2026
Keeps both CHANGELOG entries (gem manifest fix and the Poetry venv
fix from #330).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L6i4cQ51yarBFnFs2b8HRx

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 50cde0a. Configure here.

Mikola Lysenko (mikolalysenko) pushed a commit that referenced this pull request Oct 1, 2026
#330 reordered Poetry venv discovery in the same function. Keep both:
a Pipenv project uses only the venv Pipenv resolves, then Poetry's
out-of-tree env when Poetry would not use ./.venv, then the generic
.venv / venv probes.

Assisted-by: Claude Code:claude-opus-5-5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants